fix: stop search if files names are undefined #6761

emuraton · 2018-07-26T09:03:36Z

Summary

Files names seems to be undefined on certain cases for Window

      names.forEach(file => {
            ^

TypeError: Cannot read property 'forEach' of undefined
    at fs.readdir (C:\Users\timbault\talend-ui\ui\node_modules\jest-haste-map\build\crawlers\node.js:36:13)
    at go$readdir$cb (C:\Users\timbault\talend-ui\ui\node_modules\graceful-fs\graceful-fs.js:149:14)
    at FSReqWrap.oncomplete (fs.js:135:15)
error Command failed with exit code 1.

I'm not sure why this variable is undefined, but I added defensive code to return earlier more gracefuly.

Test plan

Added an unit test to confirm we return early if names is undefined.

fixes: #6715

codecov-io · 2018-07-26T10:03:56Z

Codecov Report

Merging #6761 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6761      +/-   ##
==========================================
- Coverage   66.98%   66.98%   -0.01%     
==========================================
  Files         250      250              
  Lines       10360    10363       +3     
  Branches        3        3              
==========================================
+ Hits         6940     6942       +2     
- Misses       3419     3420       +1     
  Partials        1        1

Impacted Files	Coverage Δ
packages/jest-haste-map/src/crawlers/node.js	`98.78% <ø> (-1.22%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b1e8c3...edcfe78. Read the comment docs.

thymikee · 2018-08-07T20:29:53Z

packages/jest-haste-map/src/crawlers/node.js

@@ -30,7 +30,10 @@ function find(
    activeCalls++;
    fs.readdir(directory, (err, names) => {
      activeCalls--;
-
+      if (!names) {


How about going a bit more idiomatic Node.js with if (err) {?

Yeah, if that's what happens (unhandled error) that's the correct fix. @emuraton do you have a reproduction case for your error?

@SimenB I did not suceed to replicate the issue case of @timbault.
But from the stack trace, callback doesnt return an error @thymikee, just names undefined.
I can add both check if you think its better (if( err || !names))

Doest it make sense?

If it returns undefined names, but null err, it's a bug in Node imho and you should also file it there

Agreed, the docs says you should always get an array: https://nodejs.org/api/fs.html#fs_fs_readdir_path_options_callback

Sorry for the late answer, after double thought I think you were right before actually. Checking err should be enough.
If I find a bit of time today, I'll try to replicate to see if its a bug from Node or if your first comments were right

Thanks, looking forward to it!

emuraton · 2018-08-24T14:14:24Z

After few tests cases, I always got something following this shape. So I think checking err, as you suggested, should be enough.

      { Error: ENOTDIR: not a directory, scandir '/Users/emilienmuraton/Code/jest/packages/jest-haste-map/src/__tests__/index.test.js'
        errno: -20,
        code: 'ENOTDIR',
        syscall: 'scandir',
        path: '/Users/emilienmuraton/Code/jest/packages/jest-haste-map/src/__tests__/index.test.js' }
      }
  { names: undefined }

SimenB · 2018-08-27T17:08:41Z

Could you update the changelog as well? :)

thymikee · 2018-08-29T13:04:50Z

CHANGELOG.md

@@ -9,6 +9,7 @@
 - `[babel-jest]` Make `getCacheKey()` take into account `createTransformer` options ([#6699](https://github.com/facebook/jest/pull/6699))
 - `[docs]` Fix contributors link ([#6711](https://github.com/facebook/jest/pull/6711))
 - `[website]` Fix website versions page to link to correct language ([#6734](https://github.com/facebook/jest/pull/6734))
+- `[jest-haste-map`] Catch crawler error when unsuccessfully reading directories ([#6761](https://github.com/facebook/jest/pull/6761))


You'll need to rebase/merge master, as this CHANGELOG is outdated (latest Jest release is 23.5.0)

nvm, fixed that for you :)

Thanks 👍

This reverts commit d235839.

github-actions · 2021-05-12T09:07:27Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

facebook-github-bot added the cla signed label Jul 26, 2018

emuraton changed the title ~~fix: stop search if files names are undefined~~ (WIP) fix: stop search if files names are undefined Jul 26, 2018

stop search if is undefined

d56dcf3

emuraton force-pushed the crawlers-error-window branch from a432e4b to d56dcf3 Compare July 26, 2018 09:41

emuraton changed the title ~~(WIP) fix: stop search if files names are undefined~~ fix: stop search if files names are undefined Jul 29, 2018

emuraton mentioned this pull request Aug 2, 2018

jest-haste-map crawlers error when trying to run tests on Windows #6715

Closed

thymikee reviewed Aug 7, 2018

View reviewed changes

thymikee approved these changes Aug 24, 2018

View reviewed changes

fix: callback early is readdir throw an error

0f79b06

emuraton force-pushed the crawlers-error-window branch from a1796df to 0f79b06 Compare August 24, 2018 15:01

SimenB approved these changes Aug 27, 2018

View reviewed changes

update changelog

f92bf6d

thymikee reviewed Aug 29, 2018

View reviewed changes

thymikee and others added 3 commits August 29, 2018 15:05

Merge branch 'master' into crawlers-error-window

edcfe78

Merge branch 'master' into crawlers-error-window

8f6488b

Merge branch 'master' into crawlers-error-window

fdf4e25

SimenB merged commit d235839 into jestjs:master Aug 30, 2018

rubennorte added a commit that referenced this pull request Aug 30, 2018

Revert "fix: stop search if files names are undefined (#6761)"

10353ea

This reverts commit d235839.

dependencies bot mentioned this pull request Nov 1, 2018

Update jest in / from 22.1.4 to 23.6.0 leonardofaria/leozera-ui#250

Closed

snyk-bot mentioned this pull request Mar 17, 2020

[Snyk] Upgrade expect from 23.0.0 to 23.6.0 yowainwright/react-workshop#8

Merged

dependabot bot mentioned this pull request Mar 13, 2021

Bump babel-jest from 18.0.0 to 23.6.0 CartoonIsArt/eevee#133

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: stop search if files names are undefined #6761

fix: stop search if files names are undefined #6761

emuraton commented Jul 26, 2018 •

edited

Loading

codecov-io commented Jul 26, 2018 •

edited

Loading

thymikee Aug 7, 2018

SimenB Aug 10, 2018

emuraton Aug 13, 2018

thymikee Aug 13, 2018

SimenB Aug 14, 2018

emuraton Aug 23, 2018

thymikee Aug 23, 2018

emuraton commented Aug 24, 2018 •

edited

Loading

SimenB commented Aug 27, 2018

thymikee Aug 29, 2018

thymikee Aug 29, 2018

emuraton Aug 29, 2018

github-actions bot commented May 12, 2021

fix: stop search if files names are undefined #6761

fix: stop search if files names are undefined #6761

Conversation

emuraton commented Jul 26, 2018 • edited Loading

Summary

Test plan

codecov-io commented Jul 26, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emuraton commented Aug 24, 2018 • edited Loading

SimenB commented Aug 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 12, 2021

emuraton commented Jul 26, 2018 •

edited

Loading

codecov-io commented Jul 26, 2018 •

edited

Loading

emuraton commented Aug 24, 2018 •

edited

Loading